Add Droid SDK provider#2689
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
github-actions
Bot
added
size:XXL
|
Latest Droid follow-up fixes pushed in c72f19b:
Validation: |
|
Addressed the unresolved Droid cleanup review thread in b1dabf8. Added a regression test covering two resumed Droid sessions where one Validation:
Note: the first full-suite run hit a timeout in |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces a new provider integration (Droid SDK) with significant new capability including new runtime mode, provider-specific UI behavior, and ~3k lines of new code. Two unresolved review comments identify bugs in the implementation. You can customize Macroscope's approvability policy. Learn more. |
|
Split the Droid adapter by responsibility in af7ffb0:
The Droid implementation is now 989 lines total across focused modules instead of one nearly 900-line adapter file. Validation:
|
|
Addressed the two remaining Cursor Bugbot review threads in 8939e57:
Validation:
|
|
Addressed the remaining token-usage review thread in 742ab64:
Validation after this fix:
Earlier in this pass, full |
|
Fixed the latest Cursor Bugbot thread:
Validation:
|
|
Addressed the latest Droid review batch in
Validation: |
|
Resolved the latest Bugbot findings in 969aceb.
Validation on this commit:
|
|
Resolved the latest Droid PR reports in
Validation:
|
|
Resolved the latest Macroscope comment in
Validation:
|
|
Resolved the latest Cursor Bugbot comments in
Validation:
|
| cumulativeTokenUsage: undefined, | ||
| }; | ||
| contextRef = context; | ||
| const previousContext = sessions.get(input.threadId); |
There was a problem hiding this comment.
🟢 Low Layers/DroidAdapter.ts:239
Concurrent startSession calls for the same threadId can orphan a DroidContext. At lines 239-243, closeContext(previousContext) yields during Effect.tryPromise, so two fibers can both pass the sessions.get() check, race to sessions.set(), and the loser's droid.close() is never called—neither here nor in the finalizer, which only closes contexts currently in the map.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/DroidAdapter.ts around line 239:
Concurrent `startSession` calls for the same `threadId` can orphan a `DroidContext`. At lines 239-243, `closeContext(previousContext)` yields during `Effect.tryPromise`, so two fibers can both pass the `sessions.get()` check, race to `sessions.set()`, and the loser's `droid.close()` is never called—neither here nor in the finalizer, which only closes contexts currently in the map.
Evidence trail:
apps/server/src/provider/Layers/DroidAdapter.ts line 72: `const sessions = new Map<ThreadId, DroidContext>()` — plain Map, no synchronization.
lines 118-122: `closeContext` is `Effect.tryPromise(() => { context.activeAbort?.abort(); return context.droid.close(); }).pipe(Effect.ignore)` — yields but does NOT remove entry from map.
lines 239-243: check-then-act pattern with yield point at line 241 (`yield* closeContext(previousContext)`).
lines 81-96: finalizer only closes `[...sessions.values()]` — orphaned contexts not in map are missed.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 23191e9. Configure here.
| }); | ||
| const runtimeMode = canNormalizeRuntimeMode | ||
| ? normalizeRuntimeModeForProvider(selectedProvider, rawRuntimeMode) | ||
| : rawRuntimeMode; |
There was a problem hiding this comment.
Disabled Droid shows mismatched runtime mode selector state
Medium Severity
When a user has explicitly selected the Droid provider but it becomes disabled or unavailable, resolvedUnlockedProvider is undefined, so selectedProvider falls back to "codex". Meanwhile, canNormalizeRuntimeMode is false (correctly preventing persistence of a downgrade), so runtimeMode stays as "medium-access". The ChatComposer then receives provider="codex" and runtimeMode="medium-access" — getRuntimeModeOptions("codex") excludes "medium-access", so the Select component's current value doesn't match any dropdown option. The trigger label/icon renders fine (from the config map), but the popup has no highlighted selection and the user can't re-select the current mode.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 23191e9. Configure here.
2 participants
Intention
Add Droid as a first-class T3 Code provider using Factory's TypeScript SDK: https://github.com/Factory-AI/droid-sdk-typescript
This is still WIP while we validate more real Droid permission, file, MCP, auth, and long-running streaming flows.
What this adds
droidto the shared provider, model, settings, runtime source, and driver contracts.createSession/resumeSessionfrom@factory/droid-sdkin a T3 provider adapter.gif,jpeg,png,webp) as base64 SDK image sources resolved through the existing attachment store.initResult.availableModels, including user/custom models, while preserving custom model ids so duplicate underlying models remain selectable.Implementation shape
The Droid adapter has been split by responsibility instead of keeping one large file:
DroidAdapter.ts: session lifecycle and adapter orchestration.provider/droid/DroidRuntimeEvents.ts: SDK message to T3 runtime event projection.provider/droid/DroidSdkMappings.ts: SDK model, access, reasoning, usage, approval, and user-input mappings.provider/droid/DroidAttachmentResolver.ts: attachment MIME validation and image loading.provider/droid/DroidAdapterTypes.ts: shared Droid adapter types.Security / safety notes
droid; prompt content is sent through the SDK rather than shell interpolation.Validation
droid exec --model glm-5.1 --cwd /tmp ...returneddroid-pong.HomeLab - GLM-5.1,HomeLab - Trinity-Large-Thinking,Direct - GPT-5.5-Fast-xHigh, andDirect - GPT-5.5-Low.numTurns; streaming, thinking, access-mode mapping, and custom model discovery were updated.bun fmtpassed.bun lintpassed with existing unrelated web warnings.bun typecheckpassed.cd apps/server && bun run test src/provider/Layers/DroidAdapter.test.ts src/provider/Layers/DroidProvider.test.ts.bun run testpreviously passed; after rebasing, one full parallel run hit three unrelated web timeout failures, and rerunning those exact files in isolation passed.Note
Add Droid SDK provider with session management, streaming turns, and provider-aware runtime modes
droidprovider driver (DroidDriver.ts) that manages provider snapshots with 5-minute refresh, version/installation checks, and model discovery via a short-lived SDK session.medium-accessruntime mode mapped toon-requestapproval policy andworkspace-writesandbox; Droid exposes this as an additional composer access option with provider-specific labels.TextGenerationError.resolveProviderDriverKindForInstanceSelectionnow returnsundefinedwhen the matched instance is disabled or unavailable.Macroscope summarized 23191e9.
Note
Medium Risk
Introduces a new provider driver/adapter that spawns external sessions, streams runtime events, and adds a new
RuntimeModevalue; mistakes could impact turn dispatch, approvals, or provider selection across the app.Overview
Adds Droid as a first-class provider end-to-end: new server
DroidDriver+DroidAdapterwrap@factory/droid-sdkto start/resume/stop/interrupt sessions, validate image attachments, route permission + ask-user callbacks through T3 approvals/user-input, and project SDK streaming messages into canonical runtime events (tool lifecycle, token usage, metadata, errors).Adds a Droid provider snapshot probe that checks the
droidCLI (--version) and discovers available models via a short-lived SDK session, plus registers the driver in the built-in registry and defaults/contracts/settings (DroidSettings, provider display names/default models, runtime raw-source enums).Extends
RuntimeModewithmedium-access(Droid-specific) and updates web provider selection + composer controls to show provider-specific access options, normalize incompatible modes when switching providers, and avoid resolving disabled/unavailable instance selections (with updated tests/icons/settings metadata).Reviewed by Cursor Bugbot for commit 23191e9. Bugbot is set up for automated code reviews on this repo. Configure here.